Skip to content

Conversation

@yoadria
Copy link
Contributor

@yoadria yoadria commented Aug 20, 2025

This PR solves #20

While working on the permissions issue, I noticed that when posting messages, the user who created the bridge was always listed as the author. In certain scenarios, this behaviour was illogical.
To fix this, I added two fields to “ai.bridge”:

  • user_execution_behavior: A selection field with two options: link the bridge to the user with whom it will always be executed, or execute it with the user who activates it.
  • user_predefined: Stores the default user when the first option is chosen.

All this meant that I had to handle the logic in the _process_response and _process_response_message methods.

@BinhexTeam

@yoadria yoadria force-pushed the 16.0-fix-ai_oca_bridge_permissions branch from 72356a5 to 4732946 Compare August 22, 2025 10:34
@yoadria yoadria changed the title [FIX] ai_oca_bridge premissions [IMP] ai_oca_bridge Aug 22, 2025
@yoadria yoadria force-pushed the 16.0-fix-ai_oca_bridge_permissions branch from 4732946 to 29dfab1 Compare August 22, 2025 10:38
@yoadria yoadria changed the title [IMP] ai_oca_bridge [16.0][IMP] ai_oca_bridge Aug 22, 2025
@yoadria yoadria marked this pull request as ready for review August 27, 2025 13:41
try:
response = {"id": self._get_channel().message_post(**response).id}
except Exception as e:
response = {"error": "mensaje de error para la IA"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be here....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review.
I've already fixed the mistake.

@yoadria yoadria force-pushed the 16.0-fix-ai_oca_bridge_permissions branch from 29dfab1 to 2d0cd36 Compare September 4, 2025 08:25
Copy link
Member

@etobella etobella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user_id was intended for doing this. You shouldn't replace it for a new field.

Not changing it is part of a wrong configuration. We can talk if we should improve it somehow, but we don't need this extra field for sure.

Moreover, you are adding using the payload. it is not effective. You should use create_uid for that. In any case, from a Legal perspective, is important to notice that the message comes from an AI system, so for me we shouldn't do this

@yoadria
Copy link
Contributor Author

yoadria commented Sep 8, 2025

user_id was intended for doing this. You shouldn't replace it for a new field.

Not changing it is part of a wrong configuration. We can talk if we should improve it somehow, but we don't need this extra field for sure.

Moreover, you are adding using the payload. it is not effective. You should use create_uid for that. In any case, from a Legal perspective, is important to notice that the message comes from an AI system, so for me we shouldn't do this

You're right saying that user_id shouldn't change its purpose. What I would suggest then is that it should be displayed in the view so that the user can be selected, because right now it is defaulted, see here

I mention this because currently, because with current behavior, if we wanted to link the bridge to an agent, we would have to log in with the agent's user and create the bridge, so that the bridge uses agent's user.

On the other hand, if we just show the user_id, it would solve the problem of the user the message being posted by the wrong user, and also the issue of permissions, as we control the user permissions. This would remove a big chunk of the extra logic I've added before.

@etobella
Copy link
Member

etobella commented Sep 8, 2025

Why? Just change user id in the view. Isn't it working?

@yoadria yoadria force-pushed the 16.0-fix-ai_oca_bridge_permissions branch 3 times, most recently from e66a399 to 6481c6d Compare September 15, 2025 13:05
@yoadria yoadria force-pushed the 16.0-fix-ai_oca_bridge_permissions branch from 6481c6d to accd0d1 Compare September 22, 2025 10:38
@yoadria
Copy link
Contributor Author

yoadria commented Sep 22, 2025

@etobella I believe that this solves the issue of permissions and allows you to choose the user who will manage the bridge.


def _process_response_message(self, response):
return {"id": self._get_channel().message_post(**response).id}
return {"id": self._get_channel().sudo().message_post(**response).id}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use a sudo here. If the user has no permissions, it should fail IMO

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants